Build macOS prebuilts for arm64 and x86_64 by yyy-amnezia · Pull Request #2601 · amnezia-vpn/amnezia-client

yyy-amnezia · 2026-05-14T11:38:02Z

No description provided.

ygurov

Left some comments for a single recipe though to be applied for all modified recipes

ygurov · 2026-05-15T09:35:06Z

+        return self.settings.get_safe("os.version")
+
+    def _go_env(self, goarch):
+        env = Environment()


Do not use Environment manually, or not as a main one - it ignores externally-specified flags and Conan internals passing to the Makefile.

Use an env on a top of this one instead

tc = AutotoolsToolchain(self) env = tc.environment()

ygurov · 2026-05-15T09:38:43Z

+        env = Environment()
+        env.define("GOOS", self._goos)
+        env.define("GOARCH", goarch)
+        env.define("GOROOT", self.dependencies.build["go"].package_folder)


Do not specify GOROOT - it does not change the actual go executable used, just replaces the toolchain location.
I see we have issues with passing our external go in here, so let's use this one as a first line in def generate(self). Should resolve the issue

VirtualBuildEnv(self).generate()

ygurov · 2026-05-15T09:39:57Z

+        env.define("GOARCH", goarch)
+        env.define("GOROOT", self.dependencies.build["go"].package_folder)
+        env.define("GOPATH", os.path.join(self.build_folder, "gopath"))
+        env.define("GOMODCACHE", os.path.join(self.build_folder, "gopath", "pkg", "mod"))


Hmm, specifying this one is not a bad idea!
Let's do it for all platforms, should make builds more consistent

ygurov · 2026-05-15T10:11:12Z

+        return env
+
+    def _cgo_arch_flags(self, goarch):
+        return f"-arch {self._clang_arch_map[goarch]}"


-arch option used in here allows values in apple-clang-internal format, not the golang one.
These values are corresponding with the ones used in Conan, so let's just use them

Moreover, Conan does specify multiple -arch flags itself during generate step if AutotoolsToolchain is used.

ygurov · 2026-05-15T10:14:05Z

+        env.define("GOCACHE", os.path.join(self.build_folder, "gocache"))
+        env.define("GOTELEMETRY", "off")
+        if self._macos_deployment_target:
+            env.define("MACOSX_DEPLOYMENT_TARGET", self._macos_deployment_target)


This one would be specified automatically if AutotoolsToolchain is used. I would suggest just make a proper generate step instead of applying every flag manually

ygurov · 2026-05-15T10:14:43Z

+        env.define("GOTELEMETRY", "off")
+        if self._macos_deployment_target:
+            env.define("MACOSX_DEPLOYMENT_TARGET", self._macos_deployment_target)
+        env.prepend_path("PATH", self._go_bin_path)


That's a bad idea. Use the one suggested in
https://github.com/amnezia-vpn/amnezia-client/pull/2601/changes#r3247262682

ygurov · 2026-05-15T10:17:13Z

+    def _cgo_arch_flags(self, goarch):
+        return f"-arch {self._clang_arch_map[goarch]}"
+
+    def _build_go_arch(self, goarch):


DRY
Let's use an existing build step iterating all over archs, and use lipo then

I still have a complaint about the building process - there should be a single function which does the job.
I find trying to put something into the hooks a very bad practice. Recipe becomes unclear to read and analyze in the future
DRY KISS

This one is still unresolved

ygurov · 2026-05-15T10:21:43Z

+        env.define("CGO_CFLAGS", self._cgo_arch_flags(goarch))
+        env.define("CGO_LDFLAGS", self._cgo_arch_flags(goarch))
+        with env.vars(self).apply():
+            self.run("make", env=None)


Do not run make manually, use

at = Autotools(self) at.make()

Otherwise, it would ignore all Conan internal-specified variables

ygurov

Once again, reviewed just a single recipe, but to be applied for every changed one

ygurov · 2026-05-27T23:29:26Z


+    if(APPLE AND NOT IOS AND NOT MACOS_NE)
+        set(SIGN_OVPN_SCRIPT [=[
+if [ "$CODE_SIGNING_ALLOWED" != "NO" ]; then


This is a huge hack that should be avoided at any cost - very fragile and non-transparent.
You can review the process of signing the binary from CPack cmake code, there should be more appropriate place for the code

ygurov · 2026-05-27T23:32:28Z

+    def _cgo_arch_flags(self, goarch):
+        return f"-arch {self._clang_arch_map[goarch]}"
+
+    def _build_go_arch(self, goarch):


I still have a complaint about the building process - there should be a single function which does the job.
I find trying to put something into the hooks a very bad practice. Recipe becomes unclear to read and analyze in the future
DRY KISS

ygurov · 2026-05-27T23:35:04Z

+            env.define(name, value)
+
+    def _go_arch_make_args(self, goarch):
+        return [f"{name}={value}" for name, value in self._go_cache_vars().items()] + [


Looks like a slope. Why don't we just add them by env.define() calls?
Complex logic is not what we want here
KISS

ygurov · 2026-05-27T23:36:59Z

+        }
+
+    def _define_go_cache_env(self, env):
+        for name, value in self._go_cache_vars().items():


Same for here. Why do we need all of those abstractions if we can define those vars in place at generate() stage?
KISS

ygurov · 2026-05-27T23:38:18Z

        )

    def generate(self):
+        VirtualBuildEnv(self).generate()


All recipes have virtualbuildenv by default, if it's not turned off explicitly. Redundant; safe to remove

ygurov · 2026-05-27T23:42:30Z

+
+    def _build_go_arch(self, goarch):
+        autotools = Autotools(self)
+        autotools.make(args=self._go_arch_make_args(goarch))


Subsequent calls of make with different arch could lead to UB. Make sure it's cleared or build in separate folders

This one as well

ygurov · 2026-05-27T23:44:02Z

+        return str(self.settings.os) == "Macos" and len(self._archs) > 1
+
+    @property
+    def _is_unsupported_multi_arch(self):


Property is overkill, could be inlined. KISS

ygurov · 2026-05-29T11:41:42Z

+        set(MACOS_CODE_SIGN_FLAGS "--deep")
+    endif()
+    set_target_properties(${PROJECT} PROPERTIES
+        XCODE_ATTRIBUTE_OTHER_CODE_SIGN_FLAGS "${MACOS_CODE_SIGN_FLAGS}"


Macos build could be assembled using not only the Xcode generator. I find this one an unreliable thing

ygurov · 2026-05-29T11:42:44Z

+        MACOSX_PACKAGE_LOCATION MacOS
+    )
+    target_sources(${PROJECT} PRIVATE ${MACOS_OVPN_SCRIPT})
+    if(BUILD_VPN_KEYCHAIN)


Why do we sign something during build? Please consider doing that in CPack as I mentioned previously. Or let's discuss that if you do not agree

ygurov · 2026-05-29T11:43:31Z

+    def _cgo_arch_flags(self, goarch):
+        return f"-arch {self._clang_arch_map[goarch]}"
+
+    def _build_go_arch(self, goarch):


This one is still unresolved

ygurov · 2026-05-29T11:43:39Z

+
+    def _build_go_arch(self, goarch):
+        autotools = Autotools(self)
+        autotools.make(args=self._go_arch_make_args(goarch))


This one as well

ygurov · 2026-05-29T14:54:28Z

-        ${OVPN_SCRIPTS}
-        "$<TARGET_FILE_DIR:${PROJECT}>"
-    )
+    if(NOT APPLE)


I assume on Apple it should be copied as well

ygurov · 2026-05-29T14:55:17Z


 if (APPLE AND NOT IOS AND NOT MACOS_NE)
-    list(APPEND OVPN_SCRIPTS "${CMAKE_SOURCE_DIR}/deploy/data/macos/update-resolv-conf.sh")
+    set(MACOS_OVPN_SCRIPT "${CMAKE_SOURCE_DIR}/deploy/data/macos/update-resolv-conf.sh")


Do we really need this change? :D
Looks like we use an additional variable to store a single string in the list

ygurov · 2026-05-29T14:59:56Z

+            for goarch in self._goarchs:
+                arch_build_folder = os.path.join(self.build_folder, f"build-{goarch}")
+                rmdir(self, arch_build_folder)
+                copy(self, "*", src=self.source_folder, dst=arch_build_folder, excludes=(


Looks complicated and untrivial.
I would suggest to use DESTDIR variable of the original Makefile

ygurov · 2026-05-29T15:03:54Z

+                ))
+
+                at = Autotools(self)
+                with chdir(self, arch_build_folder):


This one ain't gonna work. To execute Makefile you have to be in the same directory with it, or use -C option. The 2nd could produce unexpected output

ygurov · 2026-05-29T15:06:01Z

        env.define("GOOS", self._goos)
-        env.define("GOARCH", self._goarch)
+        if not self._is_universal_macos:
+            env.define("GOARCH", self._goarch)


You already define one in Makefile call itself. I do not think the extra one is required

ygurov · 2026-05-29T15:07:31Z

+                at = Autotools(self)
+                with chdir(self, arch_build_folder):
+                    at.make(args=[
+                        f"GOOS={self._goos}",


I do not think multi-os bundles are possible :)
Let's stick with the only one defined in env

ygurov · 2026-05-29T15:08:45Z

        tc.generate(env)

    def build(self):
+        if self._is_universal_macos:


Still special handling in here.
Let's use a unified way to compile the binary despite the fact it is multiarch. The only difference is the merging thingy made by lipo

yyy-amnezia requested a review from ygurov May 14, 2026 11:38

yyy-amnezia force-pushed the feat/macos-arm64-non-ne branch from 961b231 to 1605667 Compare May 14, 2026 15:36

ygurov requested changes May 15, 2026

View reviewed changes

yyy-amnezia force-pushed the feat/macos-arm64-non-ne branch 2 times, most recently from c079e7f to 91cc9f1 Compare May 16, 2026 09:15

akhtyamovrr mentioned this pull request May 16, 2026

MacOS app: Apple Silicon Support? #1845

Open

yyy-amnezia force-pushed the feat/macos-arm64-non-ne branch from 91cc9f1 to d1c6cd4 Compare May 19, 2026 12:38

ygurov force-pushed the feat/macos-arm64-non-ne branch 4 times, most recently from ca4a82a to 966c9c6 Compare May 25, 2026 08:10

yyy-amnezia and others added 5 commits May 26, 2026 15:27

Build macOS prebuilts for arm64 and x86_64

c62cdf8

chore: remove parallel thingy from CI/CD

ca54ddc

feat: rework amnezia-xray-bindings for multiarch

01227fb

Fix macOS universal recipe builds

cbecca4

Sign macOS OpenVPN helper script

6e7925b

yyy-amnezia force-pushed the feat/macos-arm64-non-ne branch from 966c9c6 to 6e7925b Compare May 26, 2026 13:21

ygurov requested changes May 27, 2026

View reviewed changes

Address macOS build review comments

c9befdf

ygurov requested changes May 29, 2026

View reviewed changes

yyy-amnezia added 2 commits May 29, 2026 15:31

Isolate awg-go universal builds

9b8b716

Disable Xcode signing for non-NE macOS

0a85137

ygurov requested changes May 29, 2026

View reviewed changes

Address remaining macOS recipe review comments

07b43df

Conversation

yyy-amnezia commented May 14, 2026

Uh oh!

ygurov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ygurov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants